Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add FedVSSL baseline #2412

Merged
merged 135 commits into from
Dec 18, 2023
Merged

Add FedVSSL baseline #2412

merged 135 commits into from
Dec 18, 2023

Conversation

yasar-rehman
Copy link
Contributor

@yasar-rehman yasar-rehman commented Sep 22, 2023 •
edited
Loading

Issue

#2058

Description

Related issues/PRs

Proposal

Explanation

Checklist

  • Implement proposed change
  • Write tests
  • Update documentation
  • Update changelog
  • Make CI checks pass
  • Ping maintainers on Slack (channel #contributions)

Any other comments?

yasar-rehman and others added 30 commits August 31, 2023 10:28
@yasar-rehman
Added FedVSSL
e8da06d
@yasar-rehman
updated FedVSSL
4e2cc38
@yasar-rehman
updated FedVSSL
628b2f6
@yasar-rehman
updated the FedVSSL dataset_preparation
ef9b751
add mmcv configs
b18c48c
@yasar-rehman
Update main.py
80a787c
@yasar-rehman
Update dataset_preparation.py
3578f95
@yasar-rehman
updates
186dc7c
@yasar-rehman
Merge branch 'main' of https://github.com/yasar-rehman/flower into main
6407f0c
pre-training
e17b638
create dummy data and test pre-training
122119d
@yasar-rehman
main_updated
a2f037f
@yasar-rehman
updated main
09aa607
@yasar-rehman
updated
5698b7e
upload data partitioning code
f36ab15
@yasar-rehman
updated description in dataset_preparation
14b33e6
@yasar-rehman
updated the strategy
1987c57
add parse_args
23f53e5
@yasar-rehman
updates
e68ff1e
@yasar-rehman
updated
f6e99d0
@yasar-rehman
updated
09d1349
update readme
e51f553
update
7f372f8
delete hard code for saving chkp on server
6eff888
add fedavg
ee6b11a
update
cdeafcb
update
a9af82d
update
3a987e9
update
7b24fcf
update
4c311f2
jafermarq
jafermarq reviewed Dec 8, 2023
View reviewed changes
Copy link
Contributor

@jafermarq jafermarq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @yasar-rehman, @yan-gao-GY

This is looking good. I was able to run the experiments fine. Interestingly i got much better results for UCF-101 results when pretraining on UCF-101 than those in the table...

Could you take some time to address the comments below?

baselines/fedvssl/README.md Outdated Show resolved Hide resolved
baselines/fedvssl/README.md Outdated Show resolved Hide resolved
baselines/fedvssl/README.md Outdated Show resolved Hide resolved
baselines/fedvssl/fedvssl/client.py Outdated Show resolved Hide resolved
jafermarq
jafermarq reviewed Dec 15, 2023
View reviewed changes
Copy link
Contributor

@jafermarq jafermarq left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @yasar-rehman , @yan-gao-GY ,

This baseline can be merged 🥳 . But first, could you look at the comment i left below? Ping me when ready.

In the Expected Results section it says "takes a considerable amount of time without access to server-grade hardware (~5 days with a RTX 3090)". But i thought you'd need several RTX 3090? is it 6 of them as stated earlier in the README.md?

@yan-gao-GY
Copy link
Contributor

yan-gao-GY commented Dec 15, 2023 •
edited
Loading

@jafermarq thanks for your recent fix! It would take ~1 day if using 6 RTX 3090. Let's change the statement in the Expected Results section to keep consistent?

@jafermarq jafermarq changed the title FedVSSL Add FedVSSL baseline Dec 15, 2023
jafermarq
jafermarq approved these changes Dec 15, 2023
View reviewed changes
Copy link
Contributor

@jafermarq jafermarq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yan-gao-GY , @yasar-rehman ,

Your FedVSSL baseline is ready to be merged. Many thanks for thanks for the hard work !

@jafermarq jafermarq enabled auto-merge (squash) December 18, 2023 21:59
@jafermarq jafermarq merged commit 625d1d2 into adap:main Dec 18, 2023
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yan-gao-GY yan-gao-GY yan-gao-GY left review comments

@danieljanes danieljanes danieljanes approved these changes

@jafermarq jafermarq jafermarq approved these changes

@tanertopal tanertopal Awaiting requested review from tanertopal tanertopal is a code owner

Assignees
No one assigned
Labels
summer-of-reproducibility About a baseline for Summer of Reproducibility
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yasar-rehman @yan-gao-GY @danieljanes @jafermarq